Skip to content

Route fallback blocks through per-edge trampolines so the phi matches the CFG - #18

Merged
dreamsailing59-ops merged 1 commit into
ExpansionPak:mainfrom
dougchansan:pr/llvm-fallback-block-phi
Aug 22, 2026
Merged

Route fallback blocks through per-edge trampolines so the phi matches the CFG#18
dreamsailing59-ops merged 1 commit into
ExpansionPak:mainfrom
dougchansan:pr/llvm-fallback-block-phi

Conversation

@dougchansan

Copy link
Copy Markdown
Contributor

The bug

The LLVM backend emits invalid IR on Skyward Sword (SOUE01, Broadway). All 9917 chunk objects emit, then the module verifier rejects it:

PHI node entries do not match predecessors!
  %fallback_pc = phi i32 [ %entry_pc, %cold_entry ],
                         [ -2143668204, %fallback_edge563 ], ...
  label %fallback_edge563
  label %indirect_taken562
Instruction does not dominate all uses!   (x3, cascading, on %state.0)

The verifier names %indirect_taken562 as a real predecessor of the fallback block with no entry in %fallback_pc.

Cause

emitter.cpp aliases every DOLIR_TERM_FALLBACK block onto the one shared fallback block:

if (source_.blocks[i].terminator.kind == DOLIR_TERM_FALLBACK) {
  blocks_[i] = fallback_block_;    // <- alias

That block starts with the %fallback_pc phi, and only fallbackDestination() / fallbackEdge() register an incoming value. Anything branching to blocks_[i] bypasses them — notably the switch built for DOLIR_TERM_INDIRECT — so the CFG gains a predecessor the phi does not know about.

Fix

Hand out a per-edge trampoline instead of the raw shared block, so every edge carries its own guest pc by construction rather than depending on each call site remembering to register one. fallbackDestination() becomes a thin wrapper over fallbackEdge(), which also removes the fragile GetInsertBlock()-at-call-time logic.

Why only this title

GM4E01, GLME01 and GC6E01 all build clean on the same backend. SOUE01 is 1,266,744 instructions — 1.7x Mario Kart — and Wii/Broadway code carries far more indirect branches, so it is the first to exercise the path.

Verification

  • tests/llvm_fixture.cpp gains a case covering indirect dispatch into a fallback continuation with modified state. It fails on the old emitter and passes here.
  • ctest 27/28 before and after on the same tree — the single failure was the pre-existing module_abi AArch64 enum truncation, since fixed upstream by 528a204.
  • Skyward Sword now emits 9917 objects with no verifier errors; Mario Kart still emits 5803 clean.

… the CFG

The LLVM backend emits invalid IR on Skyward Sword (SOUE01, Broadway). All 9917
chunk objects emit, then the module verifier rejects it:

  PHI node entries do not match predecessors!
    %fallback_pc = phi i32 [ %entry_pc, %cold_entry ],
                           [ -2143668204, %fallback_edge563 ], ...
    label %fallback_edge563
    label %indirect_taken562
  Instruction does not dominate all uses!   (x3, cascading, on %state.0)

emitter.cpp aliased every DOLIR_TERM_FALLBACK block onto the single shared
fallback block, whose first instruction is the fallback_pc phi. Only
fallbackDestination() and fallbackEdge() register an incoming value, so any code
branching to blocks_[i] -- notably the switch built for DOLIR_TERM_INDIRECT --
created a predecessor with no phi entry.

Hand out a per-edge trampoline instead of the raw shared block, so every edge
carries its own guest pc by construction rather than depending on each call site
remembering to register one. fallbackDestination() becomes a thin wrapper over
fallbackEdge(), which removes the fragile GetInsertBlock()-at-call-time logic.

Why only this title: GM4E01, GLME01 and GC6E01 all build clean on the same
backend. SOUE01 is 1,266,744 instructions -- 1.7x Mario Kart -- and Wii/Broadway
code carries far more indirect branches, so it is the first to exercise the path.

tests/llvm_fixture.cpp gains a case covering indirect dispatch into a fallback
continuation with modified state; it fails on the old emitter and passes here.

Verified: ctest 27/28 before and after on the same tree (the one failure was the
pre-existing module_abi AArch64 enum truncation, since fixed upstream by
528a204); Skyward Sword now emits 9917 objects with no verifier errors; Mario
Kart still emits 5803 clean.
@dreamsailing59-ops
dreamsailing59-ops merged commit 303bb12 into ExpansionPak:main Aug 22, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants